Update GitLab IDP to support OIDC #19997

enj · 2018-06-13T19:59:07Z

Update GitLab IDP to support OIDC

This change updates the GitLab IDP integration to support OpenID
Connect (while retaining support for OAuth2). This has the benefit
of using a standard that was meant for authentication. In a future
release this will allow us to remove the GitLab specific code from
our integration. The GitLab OIDC provider reuses the standard OIDC
code with a GitLab specific configuration. This change is fully
backwards compatible as the identity/user objects are unchanged.

It is not possible to detect if OIDC or OAuth2 should be used when
GitLab is configured. Thus the configuration of the IDP has a new
field called legacy which determines if OAuth2 or OIDC should be
used. If true, OAuth2 is used and if false, OIDC is used. If the
value is unspecified and the URL's host is gitlab.com, OIDC is used.
Otherwise, OAuth2 is used. In a future release, unspecified will
default to using OIDC. Eventually this flag will be removed and
only OIDC will be used. To safely use OIDC, a very recent version
of GitLab is required: version 11.1.0 or later. Attempting to use
OIDC with an earlier version will fail at login time. No data
corruption will occur.

From the perspective of the end user, nothing changes. A positive
side effect of using OIDC is that it allows us to use the openid
scope instead of the api scope. This means that OpenShift only gets
limited read access to perform authentication instead of full read
and write access to the user's GitLab account. The user may have to
reauthenticate OpenShift's OAuth client on first login.

In the future OIDC will make it easy to pull group membership
information from GitLab (the userinfo endpoint already include a
groups claim, we simply need to add code in OpenShift that uses it).

Trello: https://trello.com/c/DXntmEOV

Signed-off-by: Monis Khan mkhan@redhat.com

Fixes #19937
Fixes #17954

Supersedes #19961

/assign @simo5 @liggitt
@openshift/sig-security

enj · 2018-06-13T20:05:01Z

/hold

Needs to wait until at least 3.11
Update commit strings to match new GitLab OIDC docs
Need https://gitlab.com/gitlab-org/gitlab-ce/issues/47791#note_81269161 to be fixed, see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19784 (update PR/commit with version of GitLab required)
Update git commit message to reference legacy field (code no longer only does OIDC)
Mo needs to test this against gitlab.com (hopefully it will be at 11.1.0+ soon)
Need to decide deprecation policy around old versions of GitLab and work with @kalexand-rh to document
Document new legacy field @kalexand-rh

enj · 2018-06-13T20:05:28Z

cc @alikhajeh1

enj · 2018-06-13T20:57:52Z

pkg/oauthserver/oauth/external/gitlab/gitlab.go

+	// The ability to authenticate using GitLab, and read-only access to the user's profile information and group memberships
+	gitlabOIDCScope = "openid"
+
+	// An opaque token that uniquely identifies the user


Need to update this string to match new docs once https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19784/diffs merges.

simo5 · 2018-06-14T16:49:24Z

I'd really like to retain a way to use the old cose, should an upgrade break with the openid code

enj · 2018-06-28T16:23:19Z

@simo5 so I was unable to make any sort of OIDC to OAuth2 fallback work (the OAuth scopes are different so it would require some crazy hack to try to make that work and I am pretty sure it is not possible in general). See what you think of the current approach of legacy *bool (could be changed to apiVersion string with the same semantics via v3, oidc and "").

Currently the OAuth2 code works and the OIDC codes errors (as expected):

E0628 12:12:10.415558    3629 errorpage.go:26] AuthenticationError: incompatible gitlab IDP, ID claim is SHA256 hex digest instead of digit, claims=map[sub:HASH aud:HASH exp:1.53020245e+09 iat:1.53020233e+09 auth_time:1.529809434e+09 iss:https://gitlab.com]

simo5

Only a nit, but I'll leave to you whether to change it or not
/lgtm

simo5 · 2018-07-03T11:43:22Z

pkg/oauthserver/oauth/external/gitlab/gitlab_oidc.go

+	// https://gitlab.com/help/integration/openid_connect_provider.md
+	// Uses GitLab OIDC, requires GitLab 11.1.0 or higher
+	// Earlier versions do not work: https://gitlab.com/gitlab-org/gitlab-ce/issues/47791#note_81269161
+	gitlabAuthorizePath = "/oauth/authorize"


is there a good reason to prepend all these module variables with "gitlab" ?

Matches the existing code and the other provider integrations.

openshift-ci-robot · 2018-07-03T11:46:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, simo5

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/cmd/OWNERS~~ [enj]
~~pkg/oauthserver/OWNERS~~ [enj,simo5]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This change updates the GitLab IDP integration to support OpenID Connect (while retaining support for OAuth2). This has the benefit of using a standard that was meant for authentication. In a future release this will allow us to remove the GitLab specific code from our integration. The GitLab OIDC provider reuses the standard OIDC code with a GitLab specific configuration. This change is fully backwards compatible as the identity/user objects are unchanged. It is not possible to detect if OIDC or OAuth2 should be used when GitLab is configured. Thus the configuration of the IDP has a new field called legacy which determines if OAuth2 or OIDC should be used. If true, OAuth2 is used and if false, OIDC is used. If the value is unspecified and the URL's host is gitlab.com, OIDC is used. Otherwise, OAuth2 is used. In a future release, unspecified will default to using OIDC. Eventually this flag will be removed and only OIDC will be used. To safely use OIDC, a very recent version of GitLab is required: version 11.1.0 or later. Attempting to use OIDC with an earlier version will fail at login time. No data corruption will occur. From the perspective of the end user, nothing changes. A positive side effect of using OIDC is that it allows us to use the openid scope instead of the api scope. This means that OpenShift only gets limited read access to perform authentication instead of full read and write access to the user's GitLab account. The user may have to reauthenticate OpenShift's OAuth client on first login. In the future OIDC will make it easy to pull group membership information from GitLab (the userinfo endpoint already include a groups claim, we simply need to add code in OpenShift that uses it). Trello: https://trello.com/c/DXntmEOV Signed-off-by: Monis Khan <mkhan@redhat.com>

Signed-off-by: Monis Khan <mkhan@redhat.com>

openshift-ci-robot · 2018-07-03T18:58:39Z

New changes are detected. LGTM label has been removed.

enj · 2018-07-03T19:02:03Z

Changes were just me updating the git commit message. Retagged. Confirmed API with Jordan offline. I will remove the hold on this once gitlab.com is upgraded to v11.1.0 which should happen within the next week.

enj · 2018-07-04T18:03:15Z

/retest

enj · 2018-07-04T19:35:51Z

/retest

enj · 2018-07-05T16:35:44Z

/hold cancel

OIDC code works with gitlab.com now!

enj · 2018-07-06T02:03:35Z

/retest

enj · 2018-07-06T10:33:10Z

/retest

enj · 2018-07-06T13:43:00Z

/retest

enj · 2018-07-06T21:02:55Z

/retest

openshift-ci-robot · 2018-07-06T21:40:54Z

@enj: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/gcp	`58757d9`	link	`/test gcp`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot requested review from deads2k and smarterclayton June 13, 2018 19:59

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 13, 2018

enj mentioned this pull request Jun 13, 2018

Support both v4 and v3 GitLab User APIs #19961

Closed

enj removed request for smarterclayton and deads2k June 13, 2018 20:02

enj assigned liggitt and simo5 Jun 13, 2018

enj added the sig/security label Jun 13, 2018

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 13, 2018

enj added this to the v3.11 milestone Jun 13, 2018

enj force-pushed the enj/f/gitlab_oidc branch from e57b69e to 1365a50 Compare June 13, 2018 20:33

enj commented Jun 13, 2018

View reviewed changes

enj force-pushed the enj/f/gitlab_oidc branch from 20a854c to 807534b Compare June 28, 2018 16:15

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2018

simo5 approved these changes Jul 3, 2018

View reviewed changes

enj added 2 commits July 3, 2018 14:36

Generated

58757d9

Signed-off-by: Monis Khan <mkhan@redhat.com>

enj changed the title ~~Update GitLab IDP to use OIDC instead of OAuth2~~ Update GitLab IDP to support OIDC Jul 3, 2018

enj force-pushed the enj/f/gitlab_oidc branch from 807534b to 58757d9 Compare July 3, 2018 18:58

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2018

enj added lgtm Indicates that a PR is ready to be merged. needs-api-review labels Jul 3, 2018

enj added the api-approved label Jul 3, 2018

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 5, 2018

openshift-merge-robot merged commit 712df54 into openshift:master Jul 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update GitLab IDP to support OIDC #19997

Update GitLab IDP to support OIDC #19997

enj commented Jun 13, 2018 •

edited

Loading

enj commented Jun 13, 2018 •

edited

Loading

enj commented Jun 13, 2018

enj Jun 13, 2018

simo5 commented Jun 14, 2018

enj commented Jun 28, 2018

simo5 left a comment

simo5 Jul 3, 2018

enj Jul 3, 2018 •

edited

Loading

openshift-ci-robot commented Jul 3, 2018

openshift-ci-robot commented Jul 3, 2018

enj commented Jul 3, 2018

enj commented Jul 4, 2018

enj commented Jul 4, 2018

enj commented Jul 5, 2018

enj commented Jul 6, 2018

enj commented Jul 6, 2018

enj commented Jul 6, 2018

enj commented Jul 6, 2018

openshift-ci-robot commented Jul 6, 2018

Update GitLab IDP to support OIDC #19997

Update GitLab IDP to support OIDC #19997

Conversation

enj commented Jun 13, 2018 • edited Loading

enj commented Jun 13, 2018 • edited Loading

enj commented Jun 13, 2018

enj Jun 13, 2018

Choose a reason for hiding this comment

simo5 commented Jun 14, 2018

enj commented Jun 28, 2018

simo5 left a comment

Choose a reason for hiding this comment

simo5 Jul 3, 2018

Choose a reason for hiding this comment

enj Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

openshift-ci-robot commented Jul 3, 2018

openshift-ci-robot commented Jul 3, 2018

enj commented Jul 3, 2018

enj commented Jul 4, 2018

enj commented Jul 4, 2018

enj commented Jul 5, 2018

enj commented Jul 6, 2018

enj commented Jul 6, 2018

enj commented Jul 6, 2018

enj commented Jul 6, 2018

openshift-ci-robot commented Jul 6, 2018

enj commented Jun 13, 2018 •

edited

Loading

enj commented Jun 13, 2018 •

edited

Loading

enj Jul 3, 2018 •

edited

Loading